constructors & getters-setters #189

DarkDimius · 2014-10-13T08:33:27Z

Same as #187
This branch has the same code as in #186 but has a lot cleaner history.
Difference is several additional tests that were somehow lost in #186 and ElimLocals and Flatten being in transform package

Use ref and appliedTo instead.

The idea to traverse with currently enclosing methid is also used in LambdaLift and could be used elsewhere.

…lled. Instead of requiring to be called a given phase, change the context if that is not the case.

Lifting an application `f(arg1).f(args)` requires lifting of the whole prefix `f(arg1)`, because `f` might have a side effect.

Advantage: Can rename typed as well as untyped trees.

Previously, we determined the default getters solely from the method TermRef type. This is wrong if the method is prefix is not a path -- the prefix of the term ref will be a TypeRef and the getter selection will be a SelectFromType tree, which makes no sense.

It changes meaning drastically so should always be visible.

Some transformations encounter applications where new arguments ahve to be supplied. The method type already accounts for these argument but the Application node passed into TreeTransform#transformApply is constructed with a cpy.Apply operation which does a type assignment. That type assignment fails with a parameter mismatch unless relaxedTyping is on.

Environment parameters do not count in th eresult type.

The current formulation of lambda lift is easier to do if that's the case.

Need to drop all non-class type declarations.

Params are already added by Desugar. No special treatment needed here. Besides primaryConstructor.typeParams is always empty, because term symbols do not have type parameters. The fix turns t2660.scala into an error. I believe the error is correct, hence the test was moved with a comment to neg.

Primary constructor was picking last constructor instead of first one. This is now fixed. Also, added paramAccessors utility method.

Needed to fix a problem in CapturedVars to make this work.

Like the corresponding parameters, these also need to be rewritten to function types.

1) Type parameters are now copied to accessors 2) Constructors also work for traits 2) makes it possible do to mixin after constructors.

TypeVars can appear as keys in baseType caches. The problem is that their base types depend on their instantiation, which is not always know yet when the test is performed. So results of baseType on type variables should never be cached. Todo: check whether there are any other caching problems involving typevars.

Typevars can be parts of larger types or underlying types of other types, which renders these other types uncacheable. The new logic takes account of that.

Private fields that are accessed only from the constructor, and are accessed only after they are properly initialized are now moved into the constructor. This avoids creating a redundant objetc field. Good example: gcd in Rationals (see constrs.scala).

TypeVar is a TypeProxy.

Allow to skip labels.

…ories. We better make this configurable. Because sometimes we want to compile only the files in the immediate directory.

Should fix scala#188

When using RefinedPrinter we now have a choice whether when printing a definition tree such as def foo(x: T): U we print the parameter and result type info found in the tree or in the symbol. Previously, we printed the sym info when after typer and the tree info before. This turns out to be too inflexble. With the patch, we print the sym info if option -Yprint-syms is set, and the tree info otherwise. Also, align -Yno-deep-subtypes from camelCase to standard hyphenated option notation. Tweak where unique ids are printed.

In scalac SubstOnlyTreeMakers were implemented using substitution, and didn't actually introduce new trees. Thus there was an optimization to remove them while generating code. This optimization led to scala#190. It is now removed.

DarkDimius · 2014-10-13T11:08:35Z

I'm having a look if LabmdaLift failure for desugar.scala is due to patmat.

odersky · 2014-10-13T11:53:09Z

OK thanks! - Martin

On Mon, Oct 13, 2014 at 1:08 PM, Dmitry Petrashko notifications@github.com
wrote:

I'm having a look if LabmdaLift failure for desugar.scala is due to
patmat.

—
Reply to this email directly or view it on GitHub
#189 (comment).

Martin Odersky
EPFL

odersky · 2014-10-13T11:59:45Z

It looks like the pattern matcher generates another symbol without definition. This time it's "o51". I have enhanced the LambdaLifter to give better error messages in case something like this happens.

odersky · 2014-10-13T12:09:19Z

Note that

dotty-staging@5ee2b7e

breaks dotc_core even if LambdaLift is disabled.

odersky · 2014-10-13T12:20:16Z

More investigation shows that

 dotc Types.scala -Ycheck:patternMatcher

fails with an error

Exception in thread "main" java.lang.AssertionError: assertion failed: error at Types.scala:2633
type mismatch:
found : dotty.tools.dotc.core.Types.Type(selector195)
required: dotty.tools.dotc.core.Types.WildcardType.type
at scala.Predef$.assert(Predef.scala:165)
at dotty.tools.dotc.transform.TreeChecker$Checker.adapt(TreeChecker.scala:141)
at dotty.tools.dotc.typer.Typer$$anonfun$typed$2.apply(Typer.scala:1020)

given a pattern object b x match { case y @ b => <body> } y inside the <body> `y` should have type b.type. Inherited from scalac EqualityTestTreeMaker would reset type of it in order to achive this. Dotty instead needs to know this in advance.

DarkDimius · 2014-10-13T13:12:01Z

@odersky. Should be fixed now.

DarkDimius · 2014-10-13T13:40:40Z

It's not fixed. It breaks for more complicated patterns, I'm investigating.

DarkDimius · 2014-10-13T16:42:53Z

Ok, i've found what causes an error.
Typer creates a tree that patmat isn't able to handle:
for a pattern case a @ Assign(Ident(id), rhs)

it creates a tree that given current decoding scheme means
case a @ (Assign(Ident(id), rhs) : tpt)
Ie: type test on result of unapply. This isn't valid scala.
Raw tree is here:


Bind(a,Typed(UnApply(TypeApply(Select(Ident(Assign),unapply),List(TypeTree[TypeVar(PolyParam(dotty$tools$dotc$ast$Trees$$Assign$$unapply$$T) -> TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Untyped))])),List(),List(Typed(UnApply(TypeApply(Select(Ident(Ident),unapply),List(TypeTree[TypeVar(PolyParam(dotty$tools$dotc$ast$Trees$$Ident$$unapply$$T) -> TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Untyped))])),List(),List(Bind(id,Ident(_)))),TypeTree[RefinedType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Ident), dotty$tools$dotc$ast$Trees$$Ident$$T, ContraTypeAlias(TypeVar(PolyParam(dotty$tools$dotc$ast$Trees$$Ident$$unapply$$T) -> TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Untyped))))]), Bind(rhs,Ident(_)))),TypeTree[RefinedType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Assign), dotty$tools$dotc$ast$Trees$$Assign$$T, ContraTypeAlias(TypeVar(PolyParam(dotty$tools$dotc$ast$Trees$$Assign$$unapply$$T) -> TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Untyped))))]))

I can make a quickfix by just ignoring structure Bind(_, Typed(Unapply)).
But I am at least curious why did typer emit such code?

Substitution was creating more problems than solving. Instead the symbols in tree aren't rewriten anymore, and pattern matcher collects Rebindings and generates ValDefs for them just before the guard & body of the generated case.

Typer creates a tree that given current decoding scheme means case a @ (Assign(Ident(id), rhs) : tpt) Which isn't valid scala.

odersky · 2014-10-13T20:56:47Z

On Mon, Oct 13, 2014 at 6:42 PM, Dmitry Petrashko notifications@github.com
wrote:

Ok, i've found what causes an error.
Typer creates a tree that patmat isn't able to handle:
for a pattern case a @ Assign(Ident(id), rhs)

it creates a tree that given current decoding scheme means
case a @ (Assign(Ident(id), rhs) : Type)
Ie: type test on result of unapply. This isn't valid scala.
Raw tree is here:

Bind(a,Typed(UnApply(TypeApply(Select(Ident(Assign),unapply),List(TypeTree[TypeVar(PolyParam(dotty$tools$dotc$ast$Trees$$Assign$$unapply$$T) -> TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Untyped))])),List(),List(Typed(UnApply(TypeApply(Select(Ident(Ident),unapply),List(TypeTree[TypeVar(PolyParam(dotty$tools$dotc$ast$Trees$$Ident$$unapply$$T) -> TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Untyped))])),List(),List(Bind(id,Ident()))),TypeTree[RefinedType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Ident), dotty$tools$dotc$ast$Trees$$Ident$$T, ContraTypeAlias(TypeVar(PolyParam(dotty$tools$dotc$ast$Trees$$Ident$$unapply$$T) -> TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Untyped))))]), Bind(rhs,Ident()))),TypeTree[RefinedType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Assign), dotty$tools$dotc$ast$Trees$$Assign$$T, ContraTypeAlias(TypeVar(PolyParam(dotty$tools$
dotc$ast
$Trees$$Assign$$unapply$$T) -> TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,ast)),Trees$)),Untyped))))]))

I can make a quickfix by just ignoring structure Bind(_, Typed(Unapply)).
But I am at least curious why did typer emit such code?

The Typed(...) node gets inserted in line 741 of Applications.scala. It is
done so that we can retype the whole tree without knowledge of an expected
type. This is part of the drive to make typing idempotent.

More precisely we'd like to be in a position where after type-checking
trees, we can keep the types at the leaves of the tree, and re-type the
internal nodes using just a TypeAssigner, not a full Typer. The result of
the re-typing should be the same as the original tree. I noted that in the
case of Unapply this does not quite work yet, but can be made to work
easily, using a rule like:

assignType(tree @ Typed(unapp: Unapply, tpt) =
cpy.Typed(tree)(assignType(unapp, tpt.tpe), tpt)

That rule should be added. Without the Typed around the Unapply, assigning
a type to the Unapply would require global knowledge about the expected
type, which we do not have in the TypeAssigner architecture.

Cheers

Martin

—
Reply to this email directly or view it on GitHub
#189 (comment).

Martin Odersky
EPFL

After erasure, asInstanceworks only bedtween reference types, so erasure is not allowed to insert asInstanceOfs between numeric types. Previously this made t1260 fail even if LambdaLift is disabled.

to get a working baseline build

DarkDimius · 2014-10-22T10:32:05Z

@odersky I propose to merge this PR. WDYT?

odersky · 2014-10-22T15:39:49Z

Yes, LGTM

constructors & getters-setters

Backport "fix: hover and go to definition for named tuples" to 3.3 LTS

odersky and others added 30 commits October 11, 2014 08:24

Replace call to unimplemented method runtimeCall

00175e1

Use ref and appliedTo instead.

Facturing out EnclosingMethodTraverser from CapturedVars

7008782

The idea to traverse with currently enclosing methid is also used in LambdaLift and could be used elsewhere.

Make enteredAfter, installAfter more forgiving as to when they are ca…

8918bd9

…lled. Instead of requiring to be called a given phase, change the context if that is not the case.

Add isProperlyContainedIn method

02ab2e8

Fix lifting of application:

117b643

Lifting an application `f(arg1).f(args)` requires lifting of the whole prefix `f(arg1)`, because `f` might have a side effect.

Replace tree.withName with {tpd|untpd}.rename.tree

12a624a

Advantage: Can rename typed as well as untyped trees.

Treat @static as a source flag.

59ae5e6

It changes meaning drastically so should always be visible.

Take environment into account when typing a closure.

67e0313

Environment parameters do not count in th eresult type.

Erasure should keep prefix in termrefs.

c93f545

The current formulation of lambda lift is easier to do if that's the case.

Drop type declarations from erased ClassInfo

8975b85

Need to drop all non-class type declarations.

Uncurry also in DefDef trees, not just in infos.

d9a911c

Fix tree typing to account for trees after constructors is run.

d288981

New utility methods in SymUtils

d32ff05

Account for PolyProto and IgnoredProto types in expectedTypeStr

3f542aa

Fix to primaryConstructor

3307736

Primary constructor was picking last constructor instead of first one. This is now fixed. Also, added paramAccessors utility method.

Add constructors phase

96f754c

Combine captured vars and constructors into one macro phase.

88d11fa

Needed to fix a problem in CapturedVars to make this work.

Fix rewriting of by-name class parameter accessors.

478b97a

Like the corresponding parameters, these also need to be rewritten to function types.

Completed constructors phase

8ec91d4

1) Type parameters are now copied to accessors 2) Constructors also work for traits 2) makes it possible do to mixin after constructors.

Fix caching logic in baseTypeRef

5289a37

Typevars can be parts of larger types or underlying types of other types, which renders these other types uncacheable. The new logic takes account of that.

Added method to help traversing children in a TreeTraverser

0617380

Fix errors in SymDenotations.isCachable.

9ab1568

TypeVar is a TypeProxy.

After-rebase fix: weaken owner-chain restriction.

c0da421

Allow to skip labels.

Avoid creating ConstantTypes by TypeAssigner after erasure.

550ac13

Add possibility to compile all files in a directory and all subdirect…

f3d42cd

…ories. We better make this configurable. Because sometimes we want to compile only the files in the immediate directory.

DarkDimius and others added 4 commits October 13, 2014 11:10

Patmat: fix negation left from isEmpty&fix scoping

2f57af3

Should fix scala#188

Initial lambda lifter

0876dee

Remove special treatment of SubstOnlyTreeMakers.

5ee2b7e

In scalac SubstOnlyTreeMakers were implemented using substitution, and didn't actually introduce new trees. Thus there was an optimization to remove them while generating code. This optimization led to scala#190. It is now removed.

Disambiguate names in test case

8ae2853

Better diagnostic from LambdaLift

ac583c5

DarkDimius added 2 commits October 13, 2014 18:54

Get rid of Substitution.

5454bca

Substitution was creating more problems than solving. Instead the symbols in tree aren't rewriten anymore, and pattern matcher collects Rebindings and generates ValDefs for them just before the guard & body of the generated case.

Quickfix patmat not handling Bind(_, Typed(_: Unapply))

49cfd9f

Typer creates a tree that given current decoding scheme means case a @ (Assign(Ident(id), rhs) : tpt) Which isn't valid scala.

Test for patmat action on a @ Bind(_, Typed(_: Unapply, _)

06d278b

This was referenced Oct 14, 2014

Pattern matcher does not generate ValDefs for defined pattern bound variables #190

Closed

Pattern matcher produces scope-incorrect code. #188

Closed

odersky added 2 commits October 17, 2014 12:18

Erasure casting should perform primitive conversions

6eeb8c6

After erasure, asInstanceworks only bedtween reference types, so erasure is not allowed to insert asInstanceOfs between numeric types. Previously this made t1260 fail even if LambdaLift is disabled.

Disable LambdaLift

f27010f

to get a working baseline build

DarkDimius added a commit that referenced this pull request Oct 22, 2014

Merge pull request #189 from dotty-staging/merge

7eaee33

constructors & getters-setters

DarkDimius merged commit 7eaee33 into scala:master Oct 22, 2014

odersky mentioned this pull request Oct 26, 2014

Transform/getters setters #186

Closed

allanrenucci deleted the merge branch December 14, 2017 19:23

WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this pull request May 8, 2025

Merge pull request scala#189 from scala/backport-lts-3.3-22202

6a2dab9

Backport "fix: hover and go to definition for named tuples" to 3.3 LTS

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

constructors & getters-setters #189

constructors & getters-setters #189

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 22, 2014

Uh oh!

odersky commented Oct 22, 2014

Uh oh!

Uh oh!

constructors & getters-setters #189

constructors & getters-setters #189

Uh oh!

Conversation

DarkDimius commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 13, 2014

Uh oh!

odersky commented Oct 13, 2014

Uh oh!

DarkDimius commented Oct 22, 2014

Uh oh!

odersky commented Oct 22, 2014

Uh oh!

Uh oh!